feat: Ideogram structured-caption nodes (CORE-292)#14537
Conversation
jtydhr88
requested review from
Kosinkadink,
alexisrolland,
comfyanonymous,
guill,
kijai and
rattus128
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces four new ComfyType IO classes ( 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/color_util.py`:
- Around line 1-5: The hex_to_rgb function lacks error handling for invalid
hexadecimal characters within the 6-character string, which causes ValueError
exceptions when int() attempts to parse non-hex values like "ZZZZZZ". Wrap the
three int() conversion calls at the return statement in a try-except block that
catches ValueError, and return the same default white color tuple (255, 255,
255) when a conversion fails, ensuring graceful handling of malformed hex input
without crashing the preview render.
In `@comfy_extras/nodes_json_prompt.py`:
- Around line 52-58: The style_desc dictionary in the function is
unconditionally including aesthetics, lighting, and medium values even when they
are empty strings, which violates the documented contract that blank values
should be omitted. Modify the dictionary construction to only add these keys
(aesthetics, lighting, medium in the initial block and photo, art_style in the
conditional blocks) if their corresponding values are non-empty strings. Check
each value before adding it to style_desc to ensure only non-blank values are
included.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d2ab3530-b604-4d79-8af3-f3756db91001
📒 Files selected for processing (6)
comfy_api/latest/_io.pycomfy_extras/color_util.pycomfy_extras/nodes_bounding_boxes.pycomfy_extras/nodes_json_prompt.pycomfy_extras/nodes_string.pynodes.py
jtydhr88
force-pushed
the
feat/ideogram-prompt-nodes
branch
2 times, most recently
from
12ff69d to
c6d1759
Compare
alexisrolland
changed the title
jtydhr88
force-pushed
the
feat/ideogram-prompt-nodes
branch
from
c6d1759 to
850c913
Compare
| self.default = [] | ||
|
|
||
|
|
||
| @comfytype(io_type="BOUNDING_BOXES") |
There was a problem hiding this comment.
Would it make sense and would it be possible to reuse the existing type @comfytype(io_type="BBOX")?
There was a problem hiding this comment.
Same situation as COLORS, plus a shape mismatch.
the BOUNDING_BOXES is a concrete list[{x, y, width, height, metadata: {type, text, desc, palette}}], and it has to map to our canvas editor widget.
Two reasons we can't reuse BBOX:
- No wrap-into-array mechanism (same as COLOR): there's no backend flag or frontend path to take a single type and render/declare it as an array-valued editor widget. A dedicated @comfytype with Type = list[...] and its own Input is the only way to get the editor widget, that's BOUNDING_BOXES.
- Shape: we need a concrete per-box contract incl. metadata (type/text/desc/palette); BBOX = Any carries no shape. Note we do reuse the existing structured single type, BOUNDING_BOXES.BoundingBoxWithMetadata extends BoundingBox.BoundingBoxDict ({x, y, width, height}) and just adds metadata.
So BOUNDING_BOXES reuses the existing BOUNDING_BOX shape where it fits and exists as a distinct list type purely to drive the editor widget.
| io.String.Input("background", multiline=True, default="", | ||
| tooltip="Scene background description."), | ||
| io.DynamicCombo.Input("style", options=[ | ||
| io.DynamicCombo.Option("none", []), |
There was a problem hiding this comment.
We should remove this option, the JSON should always contain one of photo or art_style
There was a problem hiding this comment.
just double check, on KJ's version, it supports none
jtydhr88
force-pushed
the
feat/ideogram-prompt-nodes
branch
from
850c913 to
d29917b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/nodes_color.py`:
- Line 28: The change to use hex_to_rgb(color) in the ColorToRGBInt.execute
method now silently converts invalid hex digits to white instead of raising an
error, which breaks existing node behavior. Replace the hex_to_rgb call with a
stricter validation approach that preserves the original error-raising behavior
for invalid hex input, ensuring that invalid hex values are rejected rather than
defaulting to white, and that the rgb_int output remains consistent with the
actual hex input validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c1cdc31-f999-4115-bdb1-3955415268b3
📒 Files selected for processing (7)
comfy_api/latest/_io.pycomfy_extras/color_util.pycomfy_extras/nodes_bounding_boxes.pycomfy_extras/nodes_color.pycomfy_extras/nodes_json_prompt.pycomfy_extras/nodes_string.pynodes.py
🚧 Files skipped from review as they are similar to previous changes (4)
- nodes.py
- comfy_extras/nodes_json_prompt.py
- comfy_extras/color_util.py
- comfy_extras/nodes_bounding_boxes.py
| r = int(color[1:3], 16) | ||
| g = int(color[3:5], 16) | ||
| b = int(color[5:7], 16) | ||
| r, g, b = hex_to_rgb(color) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve strict invalid-hex rejection in ColorToRGBInt.execute.
On Line 28, switching to hex_to_rgb(color) makes invalid hex digits silently become white instead of raising. That changes existing node behavior and can emit mismatched outputs (rgb_int for white, hex still invalid input).
🔧 Proposed fix
def execute(cls, color: str) -> io.NodeOutput:
# expect format `#RRGGBB`
if len(color) != 7 or color[0] != "#":
raise ValueError("Color must be in format `#RRGGBB`")
+ try:
+ int(color[1:], 16)
+ except ValueError:
+ raise ValueError("Color must be in format `#RRGGBB`")
r, g, b = hex_to_rgb(color)
rgb_int = r * 256 * 256 + g * 256 + b
return io.NodeOutput(rgb_int, color)As per path instructions, comfy_extras/** changes should avoid breaking existing node interfaces.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@comfy_extras/nodes_color.py` at line 28, The change to use hex_to_rgb(color)
in the ColorToRGBInt.execute method now silently converts invalid hex digits to
white instead of raising an error, which breaks existing node behavior. Replace
the hex_to_rgb call with a stricter validation approach that preserves the
original error-raising behavior for invalid hex input, ensuring that invalid hex
values are rejected rather than defaulting to white, and that the rgb_int output
remains consistent with the actual hex input validation.
Source: Path instructions
2 participants
Two dedicated nodes for building Ideogram 4's structured JSON caption:
Adds the COMFY_DICT, COMFY_LIST, COLORS, and BOUNDING_BOXES io types;
Made COLORS and BOUNDING_BOXES so the editor widgets pass native values.
Create Bounding Boxes lives in nodes_bounding_boxes.py,
Build JSON Prompt (Ideogram) in nodes_json_prompt.py, shared color helpers in color_util.py.
Also adds a Dict to JSON String node (nodes_string.py) that serializes a COMFY_DICT with ensure_ascii=False, so non-ASCII captions render as real characters instead of \uXXXX escapes.
Screenshot
workflow
image_ideogram4_t2i (2).json
some design refer to KJ's node
need FE: Comfy-Org/ComfyUI_frontend#12960